Use the same shared FileSystem instance across calls in HadoopTableOperations#108
Use the same shared FileSystem instance across calls in HadoopTableOperations#108mccheah wants to merge 1 commit intoNetflix:masterfrom
Conversation
| } | ||
| } | ||
|
|
||
| static HadoopInputFile fromFsPath(FileSystem fs, Path path, Configuration conf) { |
There was a problem hiding this comment.
Instead of adding this, I think HadoopInputFile should make its constructor protected to allow you to create a subclass.
The reason why we use Path and Configuration is to ensure that the file system always matches the path's scheme. Adding a new factory method that allows users to supply a file system creates a path around that guarantee. That's legitimate for your use case, but you'll need to implement HadoopTableOperations anyway, so I think a safer way to solve the problem for you is to subclass HadoopInputFile and expose this method in your private subclass.
There was a problem hiding this comment.
I should note that for the limited use in this PR I think it is okay because this is package-private. It would just be better to solve your overall use case with a custom TableOperations implementation.
| HadoopTableOperations(Path location, Configuration conf) { | ||
| this.conf = conf; | ||
| this.location = location; | ||
| this.metadataFs = Util.getFS(location, conf); |
There was a problem hiding this comment.
These changes look fine to me, along with the package-private changes to HadoopInputFile and HadoopOutputFile.
|
This was migrated to apache/iceberg#14. |
2 participants
Closes #92.